Skip to content

Make SD3Transformer2DModel hidden states contiguous#14186

Merged
sayakpaul merged 4 commits into
huggingface:mainfrom
menglcai:menglcai/add_sd3_contiguous
Jul 15, 2026
Merged

Make SD3Transformer2DModel hidden states contiguous#14186
sayakpaul merged 4 commits into
huggingface:mainfrom
menglcai:menglcai/add_sd3_contiguous

Conversation

@menglcai

@menglcai menglcai commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Ensures hidden states are contiguous after each JointTransformerBlock call in SD3Transformer2DModel.

On ROCm, non-contiguous tensors produced by JointTransformerBlock cause performance degradation that accumulates across transformer blocks.

Benchmarks (SD3-medium, 512×512, 28 steps, fp16, No text_encoder_3):

Platform PyTorch Baseline Patched Speedup
AMD Radeon(TM) 8060S Graphics (gfx1151) 2.12.0+rocm7.15.0a20260711 8497 ms 7398 ms +12.9%
AMD RX 9070 XT (gfx1201) 2.12.0+rocm7.15.0a20260713 2541 ms 2348 ms +7.8%
NVIDIA RTX 5090 2.12.1+cu132 1042 ms 1017 ms within noise

Before submitting

Who can review?

@sayakpaul @yiyixuxu

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@github-actions github-actions Bot added models size/S PR with diff < 50 LOC labels Jul 14, 2026
Comment on lines +322 to +324
hidden_states = hidden_states.contiguous()
if encoder_hidden_states is not None:
encoder_hidden_states = encoder_hidden_states.contiguous()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also comment on the position in the block of code where contiguous() should be applied?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. After investigating, the non-contiguous tensor originates from PatchEmbed.forward - flatten(2).transpose(1, 2) (BCHW → BNC) produces a non-contiguous layout.

latent = latent.flatten(2).transpose(1, 2) # BCHW -> BNC

So the fix is more precisely placed right after self.pos_embed(), not inside the loop.

Updated the patch: moved .contiguous() to immediately after self.pos_embed(hidden_states), dropped the encoder_hidden_states call, and added a comment explaining the root cause.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you verify the performance table reported in the original description of this PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the numbers in the description have been updated. The re-run results are consistent with the previously reported measurements, with no significant deviation.

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@sayakpaul

Copy link
Copy Markdown
Member

Very cool work. Would be nice to also do this for others?

@sayakpaul sayakpaul merged commit 1aadc65 into huggingface:main Jul 15, 2026
13 of 15 checks passed
@menglcai

Copy link
Copy Markdown
Contributor Author

Very cool work. Would be nice to also do this for others?

I'm not sure if other models have the same issue, but will follow up and create new PR if I find cases where the similar fix applies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

models size/S PR with diff < 50 LOC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants